-
Notifications
You must be signed in to change notification settings - Fork 772
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Leverage ActivityListener.AutoGenerateRootContextTraceId #1007
Leverage ActivityListener.AutoGenerateRootContextTraceId #1007
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1007 +/- ##
==========================================
- Coverage 68.91% 68.86% -0.06%
==========================================
Files 220 220
Lines 5999 5999
Branches 984 983 -1
==========================================
- Hits 4134 4131 -3
- Misses 1596 1599 +3
Partials 269 269
|
@@ -10,7 +10,7 @@ | |||
* `BatchingActivityProcessor`/`SimpleActivityProcessor` is disposable and it | |||
disposes the containing exporter. | |||
* `BroadcastActivityProcessor`is disposable and it disposes the processors. | |||
|
|||
* Samplers now get the actual TraceId of the Activity to be created. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we need an empty line here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your favourite tool caught it as well!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚨
Just want to clarify my understanding, next OTel release will coincide with preview 8. So, will preview 8 will not contain the removal of |
Preview8 won't remove |
@cijothomas @reyang Hey I just noticed something, this broke the var parentContext = samplingParameters.ParentContext;
if (parentContext == default)
|
Fixes portions of #941 #953
Changes
Leverage ActivityListener.AutoGenerateRootContextTraceId so that sampling callbacks can get the actual TraceId of the to-be-created Activity and make decision based on TraceId.
Before this PR, Samplers generated a random TraceId, and made sampling decision. But this generated TraceID was not actually used when the Activity is created, leading to broken traces, if samplers made decision based on the TraceId.
Added Tests. There are TODOs which require follow up.
Please provide a brief description of the changes here. Update the
CHANGELOG.md
for non-trivial changes.For significant contributions please make sure you have completed the following items: